Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TT-10520] Migrating from go-redis to storage library #765

Merged
merged 27 commits into from
Jan 26, 2024
Merged

Conversation

mativm02
Copy link
Contributor

@mativm02 mativm02 commented Jan 9, 2024

Description

This PR migrates all the Redis usage to storage/temporal.

Related Issue

https://tyktech.atlassian.net/browse/TT-10520?atlOrigin=eyJpIjoiMmY4YzJkMDA1YTRkNDg4MDlmODA5Y2VhYmJmYjM5ZjYiLCJwIjoiaiJ9

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
    fork, don't request your master!
  • Make sure you are making a pull request against the master branch (left side). Also, you should start
    your branch off our latest master.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
  • I have updated the documentation accordingly.
  • Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • go fmt -s
    • go vet

Copy link

sweep-ai bot commented Jan 9, 2024

Apply Sweep Rules to your PR?

  • Apply: All new business logic should have corresponding unit tests.
  • Apply: Refactor large functions to be more modular.
  • Apply: Add docstrings to all functions and file headers.

@buger
Copy link
Member

buger commented Jan 9, 2024

API tests result - postgres15-murmur64 env: success
Branch used: refs/heads/master
Commit: 407c373 TT-10520 Migrating from go-redis to storage library (#765)

  • migrating from go-redis to storage library

  • fixing issues

  • linting

  • fixing compilation error

  • linting

  • linting part 2

  • adding constructor unit test

  • Updating storage to v1.1.0

  • Fixing GetAndDeleteSet method

  • Modifying errors to 'fatal' type

  • Running go mod tidy

  • improving tests

  • improving tests

  • linting

  • removing fatal error

  • linting

  • fixing test

  • adding TLS Options

  • linting

  • simplifying code

  • Improving code readingness

  • Making model.Connector global variable a singleton

  • Update dependencies

  • removing commented code

  • linting
    Triggered by: push (@mativm02)
    Execution page

@buger
Copy link
Member

buger commented Jan 9, 2024

API tests result - postgres15-sha256 env: success
Branch used: refs/heads/master
Commit: 407c373 TT-10520 Migrating from go-redis to storage library (#765)

  • migrating from go-redis to storage library

  • fixing issues

  • linting

  • fixing compilation error

  • linting

  • linting part 2

  • adding constructor unit test

  • Updating storage to v1.1.0

  • Fixing GetAndDeleteSet method

  • Modifying errors to 'fatal' type

  • Running go mod tidy

  • improving tests

  • improving tests

  • linting

  • removing fatal error

  • linting

  • fixing test

  • adding TLS Options

  • linting

  • simplifying code

  • Improving code readingness

  • Making model.Connector global variable a singleton

  • Update dependencies

  • removing commented code

  • linting
    Triggered by: push (@mativm02)
    Execution page

@buger
Copy link
Member

buger commented Jan 9, 2024

API tests result - mongo44-sha256 env: success
Branch used: refs/heads/master
Commit: 407c373 TT-10520 Migrating from go-redis to storage library (#765)

  • migrating from go-redis to storage library

  • fixing issues

  • linting

  • fixing compilation error

  • linting

  • linting part 2

  • adding constructor unit test

  • Updating storage to v1.1.0

  • Fixing GetAndDeleteSet method

  • Modifying errors to 'fatal' type

  • Running go mod tidy

  • improving tests

  • improving tests

  • linting

  • removing fatal error

  • linting

  • fixing test

  • adding TLS Options

  • linting

  • simplifying code

  • Improving code readingness

  • Making model.Connector global variable a singleton

  • Update dependencies

  • removing commented code

  • linting
    Triggered by: push (@mativm02)
    Execution page

@buger
Copy link
Member

buger commented Jan 9, 2024

API tests result - mongo44-murmur64 env: success
Branch used: refs/heads/master
Commit: 407c373 TT-10520 Migrating from go-redis to storage library (#765)

  • migrating from go-redis to storage library

  • fixing issues

  • linting

  • fixing compilation error

  • linting

  • linting part 2

  • adding constructor unit test

  • Updating storage to v1.1.0

  • Fixing GetAndDeleteSet method

  • Modifying errors to 'fatal' type

  • Running go mod tidy

  • improving tests

  • improving tests

  • linting

  • removing fatal error

  • linting

  • fixing test

  • adding TLS Options

  • linting

  • simplifying code

  • Improving code readingness

  • Making model.Connector global variable a singleton

  • Update dependencies

  • removing commented code

  • linting
    Triggered by: push (@mativm02)
    Execution page

Copy link

sweep-ai bot commented Jan 24, 2024

Sweeping

Resolving merge conflicts: track the progress here.

I'm currently resolving the merge conflicts in this PR. I will stack a new PR once I'm done.

Created Pull Request: #775

Comment on lines -46 to -67
t.Run("Default addresses", func(t *testing.T) {
opts := &redis.UniversalOptions{}
simpleOpts := opts.Simple()

if simpleOpts.Addr != "127.0.0.1:6379" {
t.Fatal("Wrong default single node address")
}

opts.Addrs = []string{}
clusterOpts := opts.Cluster()

if clusterOpts.Addrs[0] != "127.0.0.1:6379" || len(clusterOpts.Addrs) != 1 {
t.Fatal("Wrong default cluster mode address")
}

opts.Addrs = []string{}
failoverOpts := opts.Failover()

if failoverOpts.SentinelAddrs[0] != "127.0.0.1:26379" || len(failoverOpts.SentinelAddrs) != 1 {
t.Fatal("Wrong default sentinel mode address")
}
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test has been removed since it was testing Redis library-specific stuff - not related to the storage library

Copy link
Contributor

@tbuchaillot tbuchaillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mativm02 let's remove all the "Redis" references from structures and file names. Also double check if Redis disconnect - pump is able to handle the reconnection and monitor the number of Redis connection created ( I don't think it should be a problem with the singleton but just in case)

storage/redis.go Outdated Show resolved Hide resolved
storage/redis.go Outdated Show resolved Hide resolved
storage/redis.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tbuchaillot tbuchaillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mativm02 please add redis TLS opts and configurations

@mativm02 mativm02 requested a review from tbuchaillot January 25, 2024 14:15
Copy link
Contributor

@tbuchaillot tbuchaillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mativm02 mativm02 merged commit 407c373 into master Jan 26, 2024
22 checks passed
@mativm02 mativm02 deleted the TT-10520 branch January 26, 2024 11:37
@mativm02
Copy link
Contributor Author

/release to release-1.9

Copy link

tykbot bot commented Jan 26, 2024

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Jan 26, 2024
* migrating from go-redis to storage library

* fixing issues

* linting

* fixing compilation error

* linting

* linting part 2

* adding constructor unit test

* Updating storage to v1.1.0

* Fixing GetAndDeleteSet method

* Modifying errors to 'fatal' type

* Running go mod tidy

* improving tests

* improving tests

* linting

* removing fatal error

* linting

* fixing test

* adding TLS Options

* linting

* simplifying code

* Improving code readingness

* Making model.Connector global variable a singleton

* Update dependencies

* removing commented code

* linting

(cherry picked from commit 407c373)
Copy link

tykbot bot commented Jan 26, 2024

@mativm02 Succesfully merged PR

mativm02 added a commit that referenced this pull request Jan 29, 2024
… library (#765) (#777)

* [TT-10520] Migrating from go-redis to storage library (#765)

* migrating from go-redis to storage library

* fixing issues

* linting

* fixing compilation error

* linting

* linting part 2

* adding constructor unit test

* Updating storage to v1.1.0

* Fixing GetAndDeleteSet method

* Modifying errors to 'fatal' type

* Running go mod tidy

* improving tests

* improving tests

* linting

* removing fatal error

* linting

* fixing test

* adding TLS Options

* linting

* simplifying code

* Improving code readingness

* Making model.Connector global variable a singleton

* Update dependencies

* removing commented code

* linting

(cherry picked from commit 407c373)

* Fixing conflicts

---------

Co-authored-by: Matias <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants